Skip to content

Conversation

@weizhouapache
Copy link
Member

@weizhouapache weizhouapache commented Jun 20, 2025

Description

This PR fixes an issue when scale systemvm, when "allow.diskoffering.change.during.scale.vm" is set to "true"

refer to https://lists.apache.org/thread/0znvzpb4kftx9h1pooso63xvtzgkxzhy

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@codecov
Copy link

codecov bot commented Jun 20, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 15.18%. Comparing base (67a1ea3) to head (edd3b58).
⚠️ Report is 22 commits behind head on 4.19.

Files with missing lines Patch % Lines
...k/api/command/admin/systemvm/ScaleSystemVMCmd.java 0.00% 1 Missing ⚠️
...api/command/admin/systemvm/UpgradeSystemVMCmd.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               4.19   #11062   +/-   ##
=========================================
  Coverage     15.18%   15.18%           
  Complexity    11365    11365           
=========================================
  Files          5415     5415           
  Lines        475868   475868           
  Branches      58092    58092           
=========================================
  Hits          72252    72252           
  Misses       395531   395531           
  Partials       8085     8085           
Flag Coverage Δ
uitests 4.28% <ø> (ø)
unittests 15.90% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@weizhouapache
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13912

@weizhouapache
Copy link
Member Author

@blueorangutan test

@blueorangutan
Copy link

@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-13602)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 48039 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11062-t13602-kvm-ol8.zip
Smoke tests completed. 133 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@weizhouapache weizhouapache marked this pull request as ready for review July 10, 2025 01:56
@weizhouapache
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14126

@weizhouapache
Copy link
Member Author

@blueorangutan test keepEnv

@blueorangutan
Copy link

@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-13746)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 47800 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11062-t13746-kvm-ol8.zip
Smoke tests completed. 133 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@DaanHoogland
Copy link
Contributor

@weizhouapache , you are still working on this right?

@weizhouapache
Copy link
Member Author

@weizhouapache , you are still working on this right?

@DaanHoogland
this is ready for review/testing

@DaanHoogland DaanHoogland requested review from Copilot and removed request for Copilot July 14, 2025 11:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ensures that the details parameter for both Scale and Upgrade System VM commands is properly converted into a Map when disk offering changes are allowed during scaling.

  • Changed getDetails() in both commands to call convertDetailsToMap(details) instead of returning the raw array.
  • Applies to UpgradeSystemVMCmd and ScaleSystemVMCmd under the api module.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java Updated getDetails() to convert the raw array to a Map
api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java Updated getDetails() to convert the raw array to a Map
Comments suppressed due to low confidence (2)

api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java:73

  • Consider updating the Javadoc for getDetails to reflect that the raw details array is now converted into a Map using convertDetailsToMap, improving clarity for API consumers.
        return convertDetailsToMap(details);

api/src/main/java/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java:79

  • Add unit tests for getDetails in ScaleSystemVMCmd to verify that various detail inputs (including null and malformed entries) are correctly parsed by convertDetailsToMap.
        return convertDetailsToMap(details);

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weizhouapache , these already are Map<String, String>s. Why is the conversions from Map to Map<String, String> needed?

@weizhouapache
Copy link
Member Author

weizhouapache commented Jul 14, 2025

@weizhouapache , these already are Map<String, String>s. Why is the conversions from Map to Map<String, String> needed?

The details parameter looks like

details[0].cpunumber=2

it is not ready for use. we need to convert it to

details["cpunumber"]="2"

you may refer to #7769

cc @DaanHoogland

@DaanHoogland
Copy link
Contributor

you may refer to #7769

Ah, forgot about that one, thanks for reminding.

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@rosi-shapeblue rosi-shapeblue self-assigned this Aug 6, 2025
Copy link
Collaborator

@rosi-shapeblue rosi-shapeblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - tested and verified the PR

Test Cases Executed and Status

  1. Test Case 1: Scale Secondary Storage VM WITH Details;
  • Objective: Verify ScaleSystemVMCmd handles details parameter conversion;
  • Status: PASSED
  1. Test Case 2: Scale Console Proxy VM WITH Details;
  • Objective: Test scaling on different SystemVM type with details;
  • Status: PASSED
  1. Test Case 3: Scale Secondary Storage VM WITHOUT Details (Backward Compatibility);
  • Objective: Ensure backward compatibility without details parameter
  • Status: PASSED
  1. Test Case 4: Scale Virtual Router WITH Details;
  • Objective: Test exact scenario from original bug report;
  • Status: PASSED
  1. Test Case 5: Scale Virtual Router WITHOUT Details (Backward Compatibility)
  • Objective: Ensure VR backward compatibility without details;
  • Status: PASSED
  1. Test Case 6: Upgrade routertempalte
  • Objective Ensure router template can be upgraded without exceptions
  • Status: PASSED

Overall Success Rate: 6/6 (100%)

Logs were checked during the test cases executions - no NullPointerException was observed.

Evidence

Test Case 1: Scale Secondary Storage VM WITH Details

(localcloud) 🐱 > scale systemvm id=9e08d9a2-74bb-42af-b3ed-a7528deb4b38 serviceofferingid=9df61822-f6fa-40e6-843a-a600ad7096e6 details[0].key=test_reason details[0].value=testing_pr_11062 details[1].key=qa_engineer details[1].value=rositsa_kyuchukova
{
  "systemvm": {
    "agentstate": "Disconnected",
    "created": "2025-08-06T21:57:58+0000",
    "dns1": "10.0.32.1",
    "dns2": "8.8.8.8",
    "gateway": "10.0.48.1",
    "hasannotations": false,
    "hypervisor": "KVM",
    "id": "9e08d9a2-74bb-42af-b3ed-a7528deb4b38",
    "isdynamicallyscalable": false,
    "name": "s-1-VM",
    "podid": "8f098bb8-5990-43d6-a0f1-852970f73c2f",
    "podname": "Pod1",
    "publicip": "10.0.58.61",
    "publicmacaddress": "1e:00:a6:00:00:01",
    "publicnetmask": "255.255.240.0",
    "serviceofferingid": "9df61822-f6fa-40e6-843a-a600ad7096e6",
    "serviceofferingname": "SystemVM-Small",
    "state": "Stopped",
    "systemvmtype": "secondarystoragevm",
    "templateid": "74f2261d-730d-11f0-a1e2-1e00ef00046b",
    "templatename": "SystemVM Template (KVM)",
    "version": "4.19.4.0-SNAPSHOT",
    "zoneid": "c245525e-448f-48ed-8a8f-c6194edc6988",
    "zonename": "ref-trl-9185-k-Mol8-rositsa-kyuchukova"
  }
}

Test Case 2: Scale Console Proxy VM WITH Details

(localcloud) 🐱 > scale systemvm id=bccc6765-84ed-4091-b3c6-6ad98a31612d serviceofferingid=399e0202-738d-4b32-8bb8-a7f659653307 details[0].key=vm_type details[0].value=consoleproxy details[1].key=test_case details[1].value=scale_with_details
{
  "systemvm": {
    "activeviewersessions": 0,
    "agentstate": "Disconnected",
    "created": "2025-08-06T21:57:59+0000",
    "dns1": "10.0.32.1",
    "dns2": "8.8.8.8",
    "gateway": "10.0.48.1",
    "hasannotations": false,
    "hypervisor": "KVM",
    "id": "bccc6765-84ed-4091-b3c6-6ad98a31612d",
    "isdynamicallyscalable": false,
    "name": "v-2-VM",
    "podid": "8f098bb8-5990-43d6-a0f1-852970f73c2f",
    "podname": "Pod1",
    "publicip": "10.0.58.62",
    "publicmacaddress": "1e:00:32:00:00:02",
    "publicnetmask": "255.255.240.0",
    "serviceofferingid": "399e0202-738d-4b32-8bb8-a7f659653307",
    "serviceofferingname": "SystemVM-Medium",
    "state": "Stopped",
    "systemvmtype": "consoleproxy",
    "templateid": "74f2261d-730d-11f0-a1e2-1e00ef00046b",
    "templatename": "SystemVM Template (KVM)",
    "version": "4.19.4.0-SNAPSHOT",
    "zoneid": "c245525e-448f-48ed-8a8f-c6194edc6988",
    "zonename": "ref-trl-9185-k-Mol8-rositsa-kyuchukova"
  }
}

Test Case 3: Scale Secondary Storage VM WITHOUT Details

(localcloud) 🐱 > scale systemvm id=9e08d9a2-74bb-42af-b3ed-a7528deb4b38 serviceofferingid=399e0202-738d-4b32-8bb8-a7f659653307
{
  "systemvm": {
    "agentstate": "Disconnected",
    "created": "2025-08-06T21:57:58+0000",
    "dns1": "10.0.32.1",
    "dns2": "8.8.8.8",
    "gateway": "10.0.48.1",
    "hasannotations": false,
    "hypervisor": "KVM",
    "id": "9e08d9a2-74bb-42af-b3ed-a7528deb4b38",
    "isdynamicallyscalable": false,
    "name": "s-1-VM",
    "podid": "8f098bb8-5990-43d6-a0f1-852970f73c2f",
    "podname": "Pod1",
    "publicip": "10.0.58.61",
    "publicmacaddress": "1e:00:a6:00:00:01",
    "publicnetmask": "255.255.240.0",
    "serviceofferingid": "399e0202-738d-4b32-8bb8-a7f659653307",
    "serviceofferingname": "SystemVM-Medium",
    "state": "Stopped",
    "systemvmtype": "secondarystoragevm",
    "templateid": "74f2261d-730d-11f0-a1e2-1e00ef00046b",
    "templatename": "SystemVM Template (KVM)",
    "version": "4.19.4.0-SNAPSHOT",
    "zoneid": "c245525e-448f-48ed-8a8f-c6194edc6988",
    "zonename": "ref-trl-9185-k-Mol8-rositsa-kyuchukova"
  }
}

Test Case 4: Scale Virtual Router WITH Details

(localcloud) 🐱 > scale systemvm id=b26cfc47-c26e-4071-80c7-6f273a1ce402 serviceofferingid=9df61822-f6fa-40e6-843a-a600ad7096e6 details[0].key=test_reason details[0].value=testing_pr_11062_virtual_router details[1].key=qa_engineer details[1].value=rositsa_kyuchukova
{
  "systemvm": {
    "created": "2025-08-08T08:41:17+0000",
    "dns1": "10.0.32.1",
    "dns2": "8.8.8.8",
    "gateway": "10.0.48.1",
    "hasannotations": false,
    "hypervisor": "KVM",
    "id": "b26cfc47-c26e-4071-80c7-6f273a1ce402",
    "isdynamicallyscalable": false,
    "name": "r-4-VM",
    "podid": "8f098bb8-5990-43d6-a0f1-852970f73c2f",
    "podname": "Pod1",
    "publicip": "10.0.58.63",
    "publicmacaddress": "1e:00:fc:00:00:03",
    "publicnetmask": "255.255.240.0",
    "serviceofferingid": "9df61822-f6fa-40e6-843a-a600ad7096e6",
    "serviceofferingname": "SystemVM-Small",
    "state": "Stopped",
    "systemvmtype": "domainrouter",
    "templateid": "74f2261d-730d-11f0-a1e2-1e00ef00046b",
    "templatename": "SystemVM Template (KVM)",
    "zoneid": "c245525e-448f-48ed-8a8f-c6194edc6988",
    "zonename": "ref-trl-9185-k-Mol8-rositsa-kyuchukova"
  }
}

Test Case 5: Scale Virtual Router WITHOUT Details

(localcloud) 🐱 > scale systemvm id=b26cfc47-c26e-4071-80c7-6f273a1ce402 serviceofferingid=399e0202-738d-4b32-8bb8-a7f659653307
{
  "systemvm": {
    "created": "2025-08-08T08:41:17+0000",
    "dns1": "10.0.32.1",
    "dns2": "8.8.8.8",
    "gateway": "10.0.48.1",
    "hasannotations": false,
    "hypervisor": "KVM",
    "id": "b26cfc47-c26e-4071-80c7-6f273a1ce402",
    "isdynamicallyscalable": false,
    "name": "r-4-VM",
    "podid": "8f098bb8-5990-43d6-a0f1-852970f73c2f",
    "podname": "Pod1",
    "publicip": "10.0.58.63",
    "publicmacaddress": "1e:00:fc:00:00:03",
    "publicnetmask": "255.255.240.0",
    "serviceofferingid": "399e0202-738d-4b32-8bb8-a7f659653307",
    "serviceofferingname": "SystemVM-Medium",
    "state": "Stopped",
    "systemvmtype": "domainrouter",
    "templateid": "74f2261d-730d-11f0-a1e2-1e00ef00046b",
    "templatename": "SystemVM Template (KVM)",
    "zoneid": "c245525e-448f-48ed-8a8f-c6194edc6988",
    "zonename": "ref-trl-9185-k-Mol8-rositsa-kyuchukova"
  }
}

Test Case 6: Upgrade routertempalte

Screenshot from 2025-08-08 13-27-10

Results from logs:

tail -f /var/log/cloudstack/management/management-server.log | grep -i upgrade
2025-08-08 09:48:54,463 DEBUG [c.c.a.ApiServlet] (qtp253011924-21:[ctx-3322079d]) (logid:acd2031b) ===START===  10.0.35.125 -- GET  apiKey=LIN6rqXuaJwMPfGYFh13qDwYz5VNNz1J2J6qIOWcd3oLQOq0WtD4CwRundBL6rzXToa3lQOC_vKjI3nkHtiD8Q&command=upgradeRouterTemplate&id=3005fd3c-db83-4445-b244-c0989ef83ef7&response=json&signature=TASc72okjoYs6FkwhVuCUnrmTFA%3D
2025-08-08 09:48:54,476 DEBUG [c.c.n.r.VpcVirtualNetworkApplianceManagerImpl] (qtp253011924-21:[ctx-3322079d, ctx-62d458ee, ctx-d60beb70]) (logid:acd2031b) Router: VM instance {"id":5,"instanceName":"r-5-VM","state":"Running","type":"DomainRouter","uuid":"3005fd3c-db83-4445-b244-c0989ef83ef7"} is already at the latest version. No upgrade required
2025-08-08 09:48:54,476 DEBUG [c.c.a.ApiServlet] (qtp253011924-21:[ctx-3322079d, ctx-62d458ee, ctx-d60beb70]) (logid:acd2031b) ===END===  10.0.35.125 -- GET  apiKey=LIN6rqXuaJwMPfGYFh13qDwYz5VNNz1J2J6qIOWcd3oLQOq0WtD4CwRundBL6rzXToa3lQOC_vKjI3nkHtiD8Q&command=upgradeRouterTemplate&id=3005fd3c-db83-4445-b244-c0989ef83ef7&response=json&signature=TASc72okjoYs6FkwhVuCUnrmTFA%3D

@sureshanaparti sureshanaparti added this to the 4.19.4 milestone Aug 8, 2025
@sureshanaparti sureshanaparti merged commit 78e1462 into apache:4.19 Aug 8, 2025
25 of 26 checks passed
@weizhouapache
Copy link
Member Author

@rosi-shapeblue
Thanks for the brilliant testing

@weizhouapache weizhouapache deleted the 4.19-fix-scale-systemvm branch August 8, 2025 12:07
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants